Closed
Bug 1411660
Opened 8 years ago
Closed 8 years ago
Add web-platform tests to test-verify in automation
Categories
(Testing :: General, enhancement)
Testing
General
Tracking
(firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: gbrown, Assigned: gbrown)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
2.36 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
15.52 KB,
patch
|
jmaher
:
review+
jgraham
:
review+
|
Details | Diff | Splinter Review |
Bug 1405141 got things started by adding --verify support to the wpt harness. Now let's use it in automation. Most of this work is in mozharness.
![]() |
Assignee | |
Comment 1•8 years ago
|
||
![]() |
Assignee | |
Comment 2•8 years ago
|
||
![]() |
Assignee | |
Comment 3•8 years ago
|
||
I need to work out a couple of more issues before proceeding here, but most of that should be in mozharness -- the taskcluster stuff seems stable.
This adds a new task, "test-verify-wpt" with symbol "TVw" to desktop platforms. I'm adding this as tier 3 for now, until proven stable. This is similar to TV, but dedicated to web-platform tests. I cannot comfortably add web-platform support to the existing TV because wpt has its own mozharness script, rather than desktop_unittest.py.
Attachment #8925092 -
Flags: review?(jmaher)
Comment 4•8 years ago
|
||
Comment on attachment 8925092 [details] [diff] [review]
task cluster config changes to add test-verify-wpt
Review of attachment 8925092 [details] [diff] [review]:
-----------------------------------------------------------------
this is fairly simple!
Attachment #8925092 -
Flags: review?(jmaher) → review+
![]() |
Assignee | |
Comment 5•8 years ago
|
||
Apologies for all the delays here!
We need to finish off bug 1413729, but I'm happy with the patches that jgraham put together there. Together, this is looking good on try now, and doesn't break the existing test-verify, nor web-platform tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb7118af17fe65e32a2606cfbe323955bec7d209&filter-tier=1&filter-tier=2&filter-tier=3
Joel, looking for your review here since you've been following test verification from the start; James, you know the web-platform side.
In this patch, I'm adding web-platform test verification support to mozharness. The goal is to determine when a web-platform test is modified and then launch the test harness on each such test, with verification arguments added of course.
Attachment #8929286 -
Flags: review?(jmaher)
Attachment #8929286 -
Flags: review?(james)
Comment 6•8 years ago
|
||
Comment on attachment 8929286 [details] [diff] [review]
mozharness updates to extend test verification to web-platform tests
Review of attachment 8929286 [details] [diff] [review]:
-----------------------------------------------------------------
This took me longer than I thought to review- thanks for making this work in wpt! From what I can tell all of the odd issues we have seen when it comes to test-verify will still be accounted for in wpt which is great news.
Attachment #8929286 -
Flags: review?(jmaher) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8929286 [details] [diff] [review]
mozharness updates to extend test verification to web-platform tests
Review of attachment 8929286 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for doing this! I'm really excited to see it land. I think some followup work will be needed if we want to use this for stability checking test imports (which I would indeed like to do, since we end up with much of the same logic in the test import tool), but this is a great start.
::: testing/mozharness/mozharness/mozilla/testing/verify_tools.py
@@ +107,5 @@
> + man = wptmanifest.manifest.load(tests_root, man_path)
> +
> + repo_tests_path = os.path.join("testing", "web-platform", "tests")
> + tests_path = os.path.join("tests", "web-platform", "tests")
> + for (type, path, test) in man:
So, wpt also has a function to get a list of affected tests (and types) from a list of changed files, see [1]. We don't need to switch to that as a prerequisite to landing this, but it does try to do a little better than just checking which changed files are tests because it tries to figure out if the file is a support file that's used by some other test that will be run. Switching to that will make this work just like upstream in terms of the tests that are run, which is what we ideally want.
[1] https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wpt/testfiles.py#261-286
@@ +189,5 @@
> files = self.verify_suites.get(suite)
> references = re.compile(r"(-ref|-noref|-noref.)\.")
> for file in files:
> + if self.config.get('verify_category') == "web-platform":
> + args.append(['--verify-log-full', '--verify', file])
How does this work? Do we do a separate run per test? That seems like it isn't going to scale well; from experience with wpt imports we can sometimes have hundreds or thousands of test changes in a single push. It might be nice to (at least optionally) run all the tests in the same harness invocation to account for cases like that (in the future I would also like to be able to extend the maximum time using e.g. a try argument so that we can use this for importing wpt PRs.
Attachment #8929286 -
Flags: review?(james) → review+
![]() |
Assignee | |
Comment 8•8 years ago
|
||
(In reply to James Graham [:jgraham] from comment #7)
> So, wpt also has a function to get a list of affected tests (and types) from
> a list of changed files, see [1].
Thanks for that - I wasn't aware of it. I'm resisting the urge to switch to that immediately: filed bug 1418363.
> Do we do a separate run per test? That seems like it
> isn't going to scale well
Yes, it is a separate test harness run per test. I've filed bug 1418375 for follow-up on this and other scaling/import concerns.
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f01cc9399e6
Schedule test-verify-wpt on all desktop platforms, tier 3; r=jmaher
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b960167158f
mozharness changes to support test-verify-wpt; r=jmaher,jgraham
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5f01cc9399e6
https://hg.mozilla.org/mozilla-central/rev/5b960167158f
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•